Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new public flight view feature that allows sharing flight information from the logbook system. The changes implement a complete public flight viewing system with telemetry data visualization, flight statistics, and shareable links.
- Public flight view page with flight details, statistics, and timeline visualization
- Utility functions for flight data processing and API calls
- Integration with existing components and improved error handling
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/PublicFlightView.tsx | Complete implementation of public flight view with charts, statistics, and flight timeline |
| src/utils/publicFlight.ts | Utility functions for flight duration formatting, landing grade calculation, and phase processing |
| src/utils/fetch/publicFlight.ts | API functions for fetching public flight data and telemetry |
| src/types/publicFlight.ts | TypeScript interfaces for Flight and TelemetryPoint data structures |
| src/utils/acars.ts | ACARS message handling utilities and chart management functions |
| src/pages/Submit.tsx | Updated checkbox component usage for logbook integration |
| src/pages/Flights.tsx | Minor code formatting and component consistency improvements |
| src/components/buttons/UserButton.tsx | Changed settings link to open in new tab |
| src/components/AccessDenied.tsx | Added flight-not-found error type support |
| src/components/ACARSPanel.tsx | Removed unused ACARS panel component |
| server/db/logbook.js | Minor whitespace cleanup |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export const getLandingGrade = (fpm: number | null) => { | ||
| if (!fpm) | ||
| return { | ||
| text: 'N/A', | ||
| color: 'text-zinc-400', | ||
| bg: 'bg-gradient-to-br from-zinc-800/50 to-zinc-900/50', | ||
| border: 'border-zinc-700', | ||
| glow: '', | ||
| }; |
There was a problem hiding this comment.
The condition !fpm will be true for both null and 0. A landing rate of 0 FPM is a valid (and perfect) landing that should be graded as 'Butter', not 'N/A'. The condition should be fpm === null || fpm === undefined to only handle missing data.
| const lineGradient = nextColors | ||
| ? `bg-gradient-to-r from-${colors.baseColor} to-${nextColors.baseColor}` | ||
| : `bg-${colors.baseColor}`; |
There was a problem hiding this comment.
Dynamic Tailwind CSS class generation will not work correctly. Tailwind CSS requires class names to be present in the source code for purging. These dynamically constructed classes like bg-gradient-to-r from-yellow-500 to-orange-500 may not be included in the final CSS bundle, causing styling to fail in production.
| const lineGradient = nextColors | |
| ? `bg-gradient-to-r from-${colors.baseColor} to-${nextColors.baseColor}` | |
| : `bg-${colors.baseColor}`; | |
| // Define all possible gradient/background class names statically | |
| const phaseGradientClasses: { [key: string]: string } = { | |
| 'zinc-500_zinc-500': 'bg-zinc-500', | |
| 'zinc-500_zinc-400': 'bg-gradient-to-r from-zinc-500 to-zinc-400', | |
| 'zinc-400_zinc-500': 'bg-gradient-to-r from-zinc-400 to-zinc-500', | |
| 'yellow-500_orange-500': 'bg-gradient-to-r from-yellow-500 to-orange-500', | |
| 'orange-500_yellow-500': 'bg-gradient-to-r from-orange-500 to-yellow-500', | |
| 'yellow-500_yellow-500': 'bg-yellow-500', | |
| 'orange-500_orange-500': 'bg-orange-500', | |
| // Add all other combinations you use in phaseColor/baseColor here | |
| }; | |
| const fromColor = colors.baseColor; | |
| const toColor = nextColors?.baseColor; | |
| let lineGradient = ''; | |
| if (fromColor && toColor) { | |
| lineGradient = phaseGradientClasses[`${fromColor}_${toColor}`] || 'bg-zinc-500'; | |
| } else if (fromColor) { | |
| lineGradient = phaseGradientClasses[`${fromColor}_${fromColor}`] || 'bg-zinc-500'; | |
| } else { | |
| lineGradient = 'bg-zinc-500'; | |
| } |
| <IconComponent | ||
| className={`absolute inset-0 w-full h-full opacity-10 ${colors.text}`} | ||
| /> |
There was a problem hiding this comment.
Similar issue with dynamic class generation. The colors.text contains dynamically generated classes like text-yellow-300 that may not be preserved by Tailwind's purging process, potentially causing the styling to not work in production builds.
No description provided.